Protect against spurious failure in ctrl_c test

A failure was witnessed in the Rust repository [1] which happened right after
this test and was a problem removing a directory. Local testing confirms that if
you kill Cargo then right afterwards it's very unlikely to be able to remove the
build directory, presumably because the child process is still getting torn down
in the background.

This commit fixes the ctrl_c test itself to wait for itself to release the bulid
directory, at which point the test has definitely passed.

[1]: https://ci.appveyor.com/project/rust-lang/rust/build/1.0.1331/job/xq4ogmglj7sllibw

diff --git a/tests/death.rs b/tests/death.rs
index 52922a746e1093996397734d7e44bdaba728f64e..eb1ec07c34dc4310f3f3bb5dd65520e30fe6808a 100644 (file)
--- a/tests/death.rs
+++ b/tests/death.rs
@@ -3,9 +3,12 @@ extern crate kernel32;
 extern crate libc;
 extern crate winapi;
 
-use std::net::TcpListener;
+use std::fs;
 use std::io::{self, Read};
+use std::net::TcpListener;
 use std::process::{Stdio, Child};
+use std::thread;
+use std::time::Duration;
 
 use cargotest::support::project;
 
@@ -94,6 +97,30 @@ fn ctrl_c_kills_everyone() {
         Ok(n) => assert_eq!(n, 0),
         Err(e) => assert_eq!(e.kind(), io::ErrorKind::ConnectionReset),
     }
+
+    // Ok so what we just did was spawn cargo that spawned a build script, then
+    // we killed cargo in hopes of it killing the build script as well. If all
+    // went well the build script is now dead. On Windows, however, this is
+    // enforced with job objects which means that it may actually be in the
+    // *process* of being torn down at this point.
+    //
+    // Now on Windows we can't completely remove a file until all handles to it
+    // have been closed. Including those that represent running processes. So if
+    // we were to return here then there may still be an open reference to some
+    // file in the build directory. What we want to actually do is wait for the
+    // build script to *complete* exit. Take care of that by blowing away the
+    // build directory here, and panicking if we eventually spin too long
+    // without being able to.
+    for i in 0..10 {
+        match fs::remove_dir_all(&p.root().join("target")) {
+            Ok(()) => return,
+            Err(e) => println!("attempt {}: {}", i, e),
+        }
+        thread::sleep(Duration::from_millis(100));
+    }
+
+    panic!("couldn't remove build directory after a few tries, seems like \
+            we won't be able to!");
 }
 
 #[cfg(unix)]